[Enhancement](ai_func) Skip Null inputs in AI functions - #66242
Open
linrrzqqq wants to merge 1 commit into
Open
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Collaborator
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Request changes. The compact prompt/result mapping is functionally sound for nullable scalar arguments, and the reverse embedding-offset expansion preserves row order without copying nested vectors. Two issues remain: a legal NULL resource literal can now reach a BE DORIS_CHECK instead of propagating NULL (MAIN-002), and nullable scalar blocks with an all-zero null map unnecessarily duplicate every result (MAIN-001).
Critical checkpoint conclusions: The change is focused and covers scalar, text-embedding, and multimodal-embedding paths. There is no production concurrency, special lifecycle, configuration, persistence, transaction, storage-format, or protocol change. Return-type and overload dispatch are otherwise consistent, and provider result-count validation protects compact-row reconstruction. Tests cover partial NULLs, merged argument maps, nullable array prompts, all-NULL constants, multimodal inputs, and batch boundaries, but they miss the NULL resource and all-zero-map cases above. No extra user review focus was supplied. Per the runner instruction, this was a static review and no local build or test command was run.
linrrzqqq
force-pushed
the
ai-func-skip-null
branch
from
July 29, 2026 18:19
e02787a to
0d75fc4
Compare
Collaborator
Author
|
/review |
Contributor
There was a problem hiding this comment.
Follow-up review of 0d75fc43b7c12316bb6345091562fe9d64aa4c4b: I found no remaining blocking issue. The two previously raised concerns are addressed at this head: an all-NULL resource now returns before resource lookup, and a nullable input with an all-zero null map wraps the existing result directly instead of scattering and copying every result.
Critical checkpoint conclusions:
- Goal and correctness: The change skips NULL prompt rows for all scalar AI functions and for both text and multimodal
EMBED. Provider response counts are checked before reconstruction; scalar results are scattered in original row order, and embedding offsets are expanded in place without copying the nested Float32 vectors. - Scope and parallel paths: The change is focused on NULL propagation and compact execution. The common scalar path, String/Bool/Float result types, text embedding, multimodal embedding, constant inputs, multiple nullable arguments, all-NULL blocks, and all-zero null maps are covered.
- Nullable and compatibility contracts:
ColumnConst(ColumnNullable)handling follows the function framework's constant path, BE return nullability matches FEPropagateNullable, andEMBEDpreserves itsArray<Nullable<Float32>>element type. There is no storage, protocol, persistence, rolling-upgrade, or function-symbol compatibility change. - Concurrency, lifecycle, configuration, and data writes: There is no new production concurrency, shared mutable lifecycle, configuration item, transaction/persistence behavior, or data-write path. The test-only adapter capture is thread-local.
- Performance: NULL rows no longer consume provider requests or tokens; the scalar all-zero fast path avoids a full result copy, and the embedding path rewrites only offsets while retaining the nested vector buffer.
- Testing: BE unit coverage was added for partial NULLs, merged argument null maps, String/Bool/Float and nullable-array inputs, all-NULL constants, NULL resources, all-zero maps, text and multimodal embeddings, and batch boundaries. Per the review-runner instruction, I did not run builds or tests locally.
No additional user-provided review focus was supplied. Review completion is confirmed for the current head.
Collaborator
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29434 ms |
Contributor
TPC-DS: Total hot run time: 177519 ms |
Contributor
ClickBench: Total hot run time: 24.92 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Mryange
approved these changes
Jul 30, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem Summary:
The framework's default NULL implementation unwraps Nullable arguments and executes AI functions for every input row. For partially NULL inputs, the nested placeholder values of NULL rows are still included in prompts and sent to external AI providers.
This causes unnecessary remote requests and token consumption. It also requires special handling for embedding results to preserve the original row order without duplicating large embedding vectors.
This PR:
AIFunctionsubclasses.AIFunctionbase class.EMBED.Release note
Fix AI scalar functions to skip NULL input rows instead of sending their placeholder values to external AI providers.